Better error message and error handling for data plane gateway #916
Conversation
…are error handling
Updating license list
| 'formatjs/enforce-placeholders': 'error', | ||
| // 'formatjs/no-literal-string-in-jsx': 'error', |
There was a problem hiding this comment.
Keeping this off for right now. Will make a stand alone PR to fix all of these while turning it on.
| const dataPlaneListResponse = await dataPlaneFetcher_list( | ||
| journalClient, | ||
| journalSelector, | ||
| 'JournalData' | ||
| ); | ||
|
|
||
| if (!Array.isArray(dataPlaneListResponse)) { | ||
| return Promise.reject(dataPlaneListResponse); | ||
| } | ||
|
|
||
| return { | ||
| journals: | ||
| dataPlaneListResponse.length > 0 | ||
| ? dataPlaneListResponse | ||
| : [], | ||
| }; |
There was a problem hiding this comment.
Made these use identical variable names to make merging these together later easier.
I was going to just have the array and response in the list call but ran into typescript issues and did not want to keep trying to wrangle those.
| // There is a small chance this can happen so adding custom event to track it | ||
| // Happened before when journal data hook wasn't catching errors | ||
| const id = error.message ?? FALLBACK; | ||
| if (id === FALLBACK) { | ||
| logRocketEvent(CustomEvents.ERROR_MISSING_MESSAGE); | ||
| } |
There was a problem hiding this comment.
Make this super safe. This spot should no longer get hit - but in case making sure we have an event just for it.
We could figure this out with the event/logging above but I want to be able to search for this and not have to manually review sessions.
| // This can throw an error! Used within fetchers within SWR that is fine and SWR will handle it | ||
| // TODO (typing) | ||
| // I hate this but I need to get the bug finished | ||
| const result = await client.list(selector as any); | ||
|
|
||
| // Check for an error | ||
| if (result.err()) { | ||
| // Unwrap the error, log the error, and reject the response | ||
| const error = result.unwrap_err(); | ||
| logRocketConsole(`${key} : error : `, error); | ||
| return Promise.reject(error.body); | ||
| } | ||
|
|
||
| try { | ||
| // No error so should be fine to unwrap | ||
| const unwrappedResponse = result.unwrap(); | ||
| return await Promise.resolve(unwrappedResponse); | ||
| } catch (error: unknown) { | ||
| // This is just here to be safe. We'll keep an eye on it and possibly remove | ||
| logRocketConsole(`${key} : unwrapError : `, error); | ||
| return Promise.reject(error); | ||
| } |
There was a problem hiding this comment.
Just snagged this from the shards hook.
| @@ -183,6 +183,8 @@ const Error: ResolvedIntlConfig['messages'] = { | |||
| 'error.hintLabel': `Hint:`, | |||
| 'error.descriptionLabel': `Description:`, | |||
| 'error.tryAgain': `Try again and if the issue persists please contact support.`, | |||
|
|
|||
| 'error.fallBack': `no error details to display`, | |||
There was a problem hiding this comment.
Are we comfortable with this terse and vague of a fallback message? If so, I would recommend using sentence case here and appending with a period.
Out of curiosity, why does a generic error message -- along the lines of 'error.tryAgain' and 'errorBoundry.chunkNotFetched.error.message1' -- not work in this case?
There was a problem hiding this comment.
Yes. The point is to keep it super vague so it is not relied on too much as it is simply a failsafe.
Issues
#915
Changes
915
Misc
FormatJSlinting rulesTests
Manually tested
Automated tests
Screenshots